feat: add Windows support (updater + dictation)#323
feat: add Windows support (updater + dictation)#323Dimillian merged 42 commits intoDimillian:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ba7875e30
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if value.contains('"') { | ||
| return Err( | ||
| "Windows cmd wrapper does not support double quotes in arguments.".to_string(), | ||
| ); |
There was a problem hiding this comment.
Allow quoted args in Windows cmd wrapper
The new Windows command wrapper rejects any argument containing " (validate_cmd_token), which causes valid invocations to fail before process spawn whenever a .cmd/.bat target is used with arguments that legitimately include quotes (for example JSON payloads or pre-quoted values passed via Codex args/settings). This is a regression in Windows support because those arguments are valid for downstream tools, but the wrapper now hard-errors with Windows cmd wrapper does not support double quotes in arguments. instead of launching the command.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive Windows (x86_64) support to CodexMonitor, delivering on the milestone requirements for auto-updater and dictation functionality along with Windows UX correctness.
Changes:
- Enables Whisper-based dictation on Windows by removing the stub implementation and including required dependencies (whisper-rs, cpal) on all desktop platforms
- Implements Windows auto-updater workflow with NSIS installer bundles, signature generation, and latest.json integration
- Adds Windows UX correctness: platform-aware strings (Explorer vs Finder), Windows-friendly shortcuts (Ctrl+Alt instead of Cmd+Ctrl), cross-platform font stacks, and Windows-specific command execution wrappers for .cmd/.bat files
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils/platformPaths.ts |
New utility for platform detection and Windows-aware path handling (absolute path detection, path joining with correct separators) |
src/utils/shortcuts.ts |
Updated to display Windows-friendly shortcut labels (Ctrl+Alt+Key vs ⌘⌥Key) and handle platform differences in keyboard event matching |
src/utils/fonts.ts |
Changed to cross-platform font stacks (system-ui, Segoe UI, Cascadia Mono) instead of macOS-specific fonts |
src/features/settings/* |
Platform-aware UI strings, dynamic key labels (Option/Alt, Command/Windows/Meta), and platform-specific default shortcuts |
src/features/app/* |
Platform-conditional default "Open in" targets (macOS uses app bundles, Windows/Linux use commands); Explorer/Finder icon selection |
src-tauri/src/shared/process_core.rs |
Windows command execution utilities: resolve .cmd/.bat files, build safe cmd.exe wrappers, kill process trees with taskkill |
src-tauri/src/workspaces/commands.rs |
Windows-aware workspace opening: detect and wrap .cmd/.bat shims, use cmd /C for script execution |
src-tauri/src/backend/app_server.rs |
Cross-platform PATH building (includes Windows package managers: chocolatey, scoop, npm), Windows .cmd/.bat handling for codex binary |
src-tauri/src/dictation/mod.rs |
Removed Windows stub conditional compilation, now uses real implementation on all platforms |
src-tauri/src/dictation/stub.rs |
Deleted entire stub implementation (195 lines) |
src-tauri/src/types.rs |
Platform-conditional default shortcuts and open app targets in Rust (parallels frontend defaults) |
src-tauri/Cargo.toml |
Moved dictation dependencies (cpal, whisper-rs, sha2) from non-Windows to all desktop platforms |
src-tauri/tauri.windows.conf.json |
Enabled updater artifact creation for Windows builds |
scripts/doctor.mjs |
Cross-platform dependency checking: uses fs-based PATH scanning (no shell builtins), checks for LLVM on Windows |
.github/workflows/ci.yml |
Added Windows CI job with LLVM installation and Windows-specific build config |
.github/workflows/release.yml |
Added Windows release build job (NSIS + MSI), Windows installer detection for latest.json, artifact uploads |
docs/*, README.md, SPEC.md, PLAN.md |
Updated documentation to reflect cross-platform support (macOS + Windows + Linux) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function canExecute(filePath) { | ||
| try { | ||
| const stat = fs.statSync(filePath); | ||
| if (!stat.isFile()) return false; | ||
| if (process.platform === "win32") return true; | ||
| fs.accessSync(filePath, fs.constants.X_OK); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| function hasCommand(command) { | ||
| const checker = process.platform === "win32" ? "where" : "command"; | ||
| const checkerArgs = process.platform === "win32" ? [command] : ["-v", command]; | ||
| const result = spawnSync(checker, checkerArgs, { stdio: "ignore" }); | ||
| return result.status === 0; | ||
| const pathValue = process.env.PATH; | ||
| if (!pathValue) return false; | ||
|
|
||
| const dirs = pathValue.split(path.delimiter).filter(Boolean); | ||
|
|
||
| if (process.platform !== "win32") { | ||
| return dirs.some((dir) => canExecute(path.join(dir, command))); | ||
| } | ||
|
|
||
| const pathExtValue = process.env.PATHEXT ?? ".EXE;.CMD;.BAT;.COM"; | ||
| const exts = pathExtValue.split(";").filter(Boolean); | ||
| const hasExtension = path.extname(command) !== ""; | ||
|
|
||
| for (const dir of dirs) { | ||
| if (hasExtension && canExecute(path.join(dir, command))) return true; | ||
| for (const ext of exts) { | ||
| if (canExecute(path.join(dir, `${command}${ext}`))) return true; | ||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
The canExecute function returns true for all files on Windows (line 10), regardless of whether they're actually executable. This is because Windows doesn't use Unix-style execute permissions. However, this means the function will return true for any file (including .txt, .dll, etc.) on Windows.
While this is probably acceptable for the PATH-based command detection (since PATHEXT filtering happens in hasCommand), the function name canExecute is misleading. Consider renaming it to something like isFileAndExecutable or isFileForPlatform, or split it into platform-specific implementations to make the behavior clearer.
Additionally, on Windows at line 33-34, the function checks if a file with an extension exists even when iterating through PATHEXT extensions, which means it might check "cmake.exe.EXE" if someone provides "cmake.exe" as input. This is likely harmless but worth noting.
| #[cfg(target_os = "windows")] | ||
| let mut cmd = { | ||
| let resolved = resolve_windows_executable(trimmed, None); | ||
| let resolved_path = resolved | ||
| .as_deref() | ||
| .unwrap_or_else(|| Path::new(trimmed)); | ||
| let ext = resolved_path | ||
| .extension() | ||
| .and_then(|ext| ext.to_str()) | ||
| .map(|ext| ext.to_ascii_lowercase()); | ||
|
|
||
| if matches!(ext.as_deref(), Some("cmd") | Some("bat")) || ext.is_none() { | ||
| let mut cmd = tokio_command("cmd"); | ||
| let mut command_args = args.clone(); | ||
| command_args.push(path.clone()); | ||
| let command_line = build_cmd_c_command(resolved_path, &command_args)?; | ||
| cmd.arg("/D"); | ||
| cmd.arg("/S"); | ||
| cmd.arg("/C"); | ||
| cmd.arg(command_line); | ||
| cmd | ||
| } else { | ||
| let mut cmd = tokio_command(resolved_path); | ||
| cmd.args(&args).arg(&path); | ||
| cmd | ||
| } | ||
| }; |
There was a problem hiding this comment.
On line 840, the condition || ext.is_none() means that if a Windows executable has no extension, it will be wrapped with cmd /C. This seems incorrect - executables without extensions (though rare on Windows) should be executed directly, not through cmd.
The current logic will treat codex (with no .exe extension) as a cmd script even if it's a valid executable. While resolve_windows_executable on line 831 should find codex.exe if it exists in PATH, if the user explicitly provides a path to an extensionless executable, this will fail.
Consider changing line 840 to only wrap .cmd and .bat files, not executables without extensions. If an extensionless file needs to be executed through cmd, that should be the user's responsibility to configure.
| export function isMacPlatform(): boolean { | ||
| return platformKind() === "mac"; | ||
| } |
There was a problem hiding this comment.
There are two different implementations of isMacPlatform() in the codebase:
- In
src/utils/shortcuts.ts(lines 176-181): Uses/Mac|iPhone|iPad|iPod/.test(navigator.platform) - In
src/utils/platformPaths.ts(lines 23-25): UsesplatformKind() === "mac"which checks if platform string includes "mac"
These implementations differ in their detection logic and could produce inconsistent results. The shortcuts.ts implementation checks for specific Apple devices, while platformPaths.ts uses a more general substring match.
Recommendation: Consolidate to a single implementation. Since shortcuts.ts already exports isMacPlatform(), consider importing it from platformPaths.ts instead of re-implementing it. Alternatively, move the implementation to platformPaths.ts and have shortcuts.ts import from there, as platformPaths.ts seems like the more appropriate location for platform detection utilities.
| > | ||
| <span className="home-icon" aria-hidden> | ||
| ⌘ | ||
| <FolderOpen size={18} /> |
There was a problem hiding this comment.
The Home component icon has been changed from the Command symbol (⌘) to a FolderOpen icon. While this makes the button more platform-agnostic (the Command symbol is macOS-specific), this change seems unrelated to Windows support and appears to be a UX improvement that could have been a separate change.
The icon change makes sense from a platform-agnostic perspective, but note that this changes the visual appearance on all platforms, not just Windows. Ensure this aligns with the intended design.
| <FolderOpen size={18} /> | |
| {typeof navigator !== "undefined" && | |
| /Mac/i.test(navigator.platform) | |
| ? "⌘" | |
| : <FolderOpen size={18} />} |
| .settings-select option { | ||
| background-color: var(--surface-popover); | ||
| color: var(--text-strong); | ||
| } |
There was a problem hiding this comment.
The new CSS rule for .settings-select option sets background and text colors to improve readability in dropdown menus on Windows. However, this rule uses CSS variables that may not be well-defined or may have platform-specific rendering issues:
var(--surface-popover)andvar(--text-strong)need to be verified across all themes (system, light, dark, dim)- Some browsers/platforms may ignore these styles for
<option>elements (native styling varies significantly) - Consider adding a comment explaining why this is needed (likely for Windows dark mode rendering)
Test this thoroughly on Windows with all theme combinations to ensure the dropdown remains usable.
| export const DEFAULT_OPEN_APP_ID = isWindowsPlatform() ? "finder" : "vscode"; | ||
|
|
||
| export type OpenAppId = string; | ||
|
|
||
| export const DEFAULT_OPEN_APP_TARGETS: OpenAppTarget[] = [ | ||
| { | ||
| id: "vscode", | ||
| label: "VS Code", | ||
| kind: "app", | ||
| appName: "Visual Studio Code", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "cursor", | ||
| label: "Cursor", | ||
| kind: "app", | ||
| appName: "Cursor", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "zed", | ||
| label: "Zed", | ||
| kind: "app", | ||
| appName: "Zed", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "ghostty", | ||
| label: "Ghostty", | ||
| kind: "app", | ||
| appName: "Ghostty", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "antigravity", | ||
| label: "Antigravity", | ||
| kind: "app", | ||
| appName: "Antigravity", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "finder", | ||
| label: "Finder", | ||
| kind: "finder", | ||
| args: [], | ||
| }, | ||
| ]; | ||
| export const DEFAULT_OPEN_APP_TARGETS: OpenAppTarget[] = isMacPlatform() | ||
| ? [ | ||
| { | ||
| id: "vscode", | ||
| label: "VS Code", | ||
| kind: "app", | ||
| appName: "Visual Studio Code", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "cursor", | ||
| label: "Cursor", | ||
| kind: "app", | ||
| appName: "Cursor", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "zed", | ||
| label: "Zed", | ||
| kind: "app", | ||
| appName: "Zed", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "ghostty", | ||
| label: "Ghostty", | ||
| kind: "app", | ||
| appName: "Ghostty", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "antigravity", | ||
| label: "Antigravity", | ||
| kind: "app", | ||
| appName: "Antigravity", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "finder", | ||
| label: fileManagerName(), | ||
| kind: "finder", | ||
| args: [], | ||
| }, | ||
| ] | ||
| : [ | ||
| { | ||
| id: "vscode", | ||
| label: "VS Code", | ||
| kind: "command", | ||
| command: "code", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "cursor", | ||
| label: "Cursor", | ||
| kind: "command", | ||
| command: "cursor", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "zed", | ||
| label: "Zed", | ||
| kind: "command", | ||
| command: "zed", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "ghostty", | ||
| label: "Ghostty", | ||
| kind: "command", | ||
| command: "ghostty", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "antigravity", | ||
| label: "Antigravity", | ||
| kind: "command", | ||
| command: "antigravity", | ||
| args: [], | ||
| }, | ||
| { | ||
| id: "finder", | ||
| label: fileManagerName(), | ||
| kind: "finder", | ||
| args: [], | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
The constants DEFAULT_OPEN_APP_TARGETS and DEFAULT_OPEN_APP_ID are evaluated at module load time. This means the platform detection happens once when the module is first imported, not dynamically at runtime.
If the application is built on one platform and distributed as a universal build (though unlikely for Tauri), or if there are SSR/testing scenarios, this could lead to incorrect defaults. Consider either:
- Wrapping these in getter functions that evaluate the platform on each call
- Documenting that these are build-time constants
- Ensuring the build process creates platform-specific bundles
Since this is a Tauri app with platform-specific builds, this is likely acceptable, but it should be consistent with the approach used elsewhere in the codebase.
| fn validate_cmd_token(value: &str) -> Result<(), String> { | ||
| if value.contains('\0') { | ||
| return Err("Windows cmd wrapper does not support NUL bytes.".to_string()); | ||
| } | ||
| if value.contains('\n') || value.contains('\r') { | ||
| return Err("Windows cmd wrapper does not support newline characters.".to_string()); | ||
| } | ||
| if value.contains('"') { | ||
| return Err( | ||
| "Windows cmd wrapper does not support double quotes in arguments.".to_string(), | ||
| ); | ||
| } | ||
| Ok(()) | ||
| } |
There was a problem hiding this comment.
The validate_cmd_token function rejects arguments containing double quotes, which is a security measure to prevent command injection. However, this means legitimate file paths or arguments containing quotes cannot be passed through this wrapper.
While this is a reasonable security trade-off for the current use case (opening editors/apps with workspace paths), consider documenting this limitation. Users trying to configure custom commands with quoted arguments will receive a somewhat cryptic error message about "not supporting double quotes."
Consider either:
- Adding a more user-friendly error message that explains why quotes are rejected
- Implementing proper escaping for quotes within the cmd.exe context (though this is complex and error-prone)
- Documenting this limitation in the UI where users configure custom commands
| function looksLikeWindowsAbsolutePath(value: string): boolean { | ||
| if (/^[A-Za-z]:[\\/]/.test(value)) { | ||
| return true; | ||
| } | ||
| if (value.startsWith("\\\\") || value.startsWith("//")) { | ||
| return true; | ||
| } | ||
| if (value.startsWith("\\\\?\\")) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
The looksLikeWindowsAbsolutePath function on line 58 uses a regex /^[A-Za-z]:[\\/]/ to detect Windows drive letters. This pattern requires exactly one character for the drive letter, but Windows only accepts A-Z or a-z as valid drive letters. While the current regex is correct, consider making it more explicit with a comment or using a more restrictive pattern like /^[A-Za-z]:(?:[\\/]|$)/ to also handle bare drive references like "C:" (though this may not be needed for the current use case).
Also note that the function will return true for UNC paths starting with // (forward slashes), which is technically correct as Windows accepts both forward and backward slashes, but this might be worth documenting.
| const isMac = isMacPlatform(); | ||
| const metaMatches = parsed.meta | ||
| ? isMac | ||
| ? event.metaKey | ||
| : event.ctrlKey || event.metaKey | ||
| : !event.metaKey; | ||
| if (!metaMatches) { | ||
| return false; | ||
| } | ||
|
|
||
| const ctrlMatches = parsed.ctrl | ||
| ? event.ctrlKey | ||
| : parsed.meta && !isMac | ||
| ? true | ||
| : !event.ctrlKey; | ||
| return ctrlMatches && parsed.alt === event.altKey && parsed.shift === event.shiftKey; |
There was a problem hiding this comment.
The matchesShortcut function has complex logic to handle platform differences between macOS and Windows. Specifically, lines 168-172 implement special handling where:
- On non-Mac platforms, if
parsed.ctrlis true, it requiresevent.ctrlKey - But if
parsed.metais true on non-Mac (line 170), it setsctrlMatches = trueregardless of the actual ctrl key state
This means on Windows, a shortcut defined as "cmd+shift+m" (parsed.meta=true) will match when the user presses Ctrl+Shift+M (because line 162 accepts event.ctrlKey for meta shortcuts on non-Mac). However, line 170-171 then ignores the ctrl key state entirely and always returns true for ctrlMatches.
This could lead to unexpected behavior: if a user has BOTH Ctrl and Meta keys pressed on a non-Mac platform for a "cmd" shortcut, the function would still match. While this may be intentional to handle platform quirks, it should be documented or simplified.
| for candidate in exe_candidates: | ||
| lowered = candidate.name.lower() | ||
| if "setup" in lowered or "installer" in lowered: | ||
| windows_installer = candidate | ||
| break | ||
| if windows_installer is None and exe_candidates: | ||
| windows_installer = exe_candidates[0] | ||
| if windows_installer is None: | ||
| raise SystemExit("No Windows installer (.exe) found for latest.json") | ||
|
|
There was a problem hiding this comment.
The Windows installer detection logic searches for .exe files containing "setup" or "installer" in lowercase filenames, with a fallback to the first .exe found if no match. This heuristic approach could be fragile if:
- Tauri changes its naming convention for Windows installers
- Multiple .exe files are present in the release artifacts with ambiguous names
- A debug or test .exe is alphabetically first
Consider either:
- Using a more specific pattern that matches Tauri's naming convention (e.g., checking for the app name in the filename)
- Having Tauri output a manifest file listing the updater bundle filename
- Using a glob pattern that's more specific to the actual output (e.g.,
*_x64-setup.exeor similar) - Adding validation that the selected file has an expected size or signature format
For robustness, at minimum add a log message showing which installer was selected.
| for candidate in exe_candidates: | |
| lowered = candidate.name.lower() | |
| if "setup" in lowered or "installer" in lowered: | |
| windows_installer = candidate | |
| break | |
| if windows_installer is None and exe_candidates: | |
| windows_installer = exe_candidates[0] | |
| if windows_installer is None: | |
| raise SystemExit("No Windows installer (.exe) found for latest.json") | |
| # Prefer executables that look like the CodexMonitor installer | |
| preferred_installers = [] | |
| for candidate in exe_candidates: | |
| lowered = candidate.name.lower() | |
| if "codexmonitor" in lowered and ("setup" in lowered or "installer" in lowered): | |
| preferred_installers.append(candidate) | |
| if preferred_installers: | |
| windows_installer = preferred_installers[0] | |
| else: | |
| # Fallback to any executable containing "setup" or "installer" | |
| for candidate in exe_candidates: | |
| lowered = candidate.name.lower() | |
| if "setup" in lowered or "installer" in lowered: | |
| windows_installer = candidate | |
| break | |
| if windows_installer is None and exe_candidates: | |
| # Final fallback: first .exe alphabetically (for backward compatibility) | |
| windows_installer = exe_candidates[0] | |
| if windows_installer is None: | |
| raise SystemExit("No Windows installer (.exe) found for latest.json") | |
| print(f"Selected Windows installer for latest.json: {windows_installer.name}") |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ba7875e30
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| release-artifacts/nsis/*.exe* \ | ||
| release-artifacts/msi/*.msi* \ |
There was a problem hiding this comment.
Reference Windows release assets with stable glob paths
In the release job I checked, Windows artifacts are downloaded from windows-artifacts into release-artifacts, but gh release create only looks under release-artifacts/nsis/*.exe* and release-artifacts/msi/*.msi*; this path assumption is brittle compared with the rest of the workflow (which consumes downloaded artifacts from the root), and when those subfolders are not present the glob is passed as a literal and gh release create fails, blocking publication of the release.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 44 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@Dimillian reviews look clean locally and from Copilot and Codex. Ready for your review again! |
|
@amanthanvi can you check my PR? yours is probably more complete, but just in case there something that can be added (I had trouble with threads loading + terminal , did you get these?) |
|
FYI I'll merge this soon |
# Conflicts: # src/features/git/components/GitDiffPanel.test.tsx # src/features/git/components/GitDiffPanel.tsx
Co-authored-by: Thomas Ricouard <ricouard77@gmail.com>
Co-authored-by: Thomas Ricouard <ricouard77@gmail.com>
Closes #340
Related: #4
Summary
Add end-to-end Windows (x86_64) support for CodexMonitor per
SPEC.md/PLAN.md, including the milestone-required auto-updater + dictation work and Windows UX correctness/polish.Key changes
Dictation on Windows (required)
doctor:win) for CMake + LLVM/Clang (bindgen/libclang).Updater + release workflow on Windows (required)
src-tauri/tauri.windows.conf.json..exe/NSIS +.msi) plus updater signatures.latest.jsongeneration includeswindows-x86_64pointing at the NSIS installer (.exe+.exe.sig).Windows UX correctness
.cmd/.bat, e.g.code.cmd) usingcmd /D /S /Cwith a single fully-quoted command line.Cmd+Ctrlcollapsing intoCtrlon Windows) and shortcut display polish.Testing
Local (Linux):
npm run lintnpm run testnpm run typecheckWindows (x86_64):
npm run tauri:build:winbuilds and produces NSIS + MSI installers.Notes
TAURI_SIGNING_PRIVATE_KEYto be set; CI/release workflows inject it fromTAURI_SIGNING_PRIVATE_KEY_B64secrets.AI assistance disclosure
This PR and its changes were produced with AI assistance: OpenAI GPT‑5.2 via Codex CLI. All changes were reviewed/curated before submission.